feat(nextjs,react): Add HandleSSOCallback component #7678
Conversation
🦋 Changeset detectedLatest commit: 433b4fe The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughBumps minor versions for Clerk packages (chrome-extension, nextjs, react). Introduces a new exported React component, HandleSSOCallback, and its props interface, which orchestrates SSO callback flows (sign-in, sign-up, transfer paths, session activation) and accepts navigateToApp, navigateToSignIn, navigateToSignUp callbacks plus an optional render prop. Exposes the component on the Next.js package surface and in the Chrome Extension re-exports. The component runs its flow once to accommodate SSR/Next.js re-renders. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
| {/* Because a sign-in transferred to a sign-up might require captcha verification, make sure to render the | ||
| captcha element. */} |
There was a problem hiding this comment.
should we allow children for them to render a spinner here possibly?
There was a problem hiding this comment.
I went with the render prop for that, but I'll defer to what you suggest here since I know the thinking has evolved recently!
There was a problem hiding this comment.
to date we've used children, so I might lean towards children for now. We've yet to introduce a render prop but possibly in the future.
There was a problem hiding this comment.
stepping back do we even need either? This renders a div and the captcha (possibly). wouldn't folks probably write code that looks like this?
<Card>
<CardBody>
<Spinner />
<HandleSSOCallback />
</CardBody>
</Card>
In this situation I don't think they'd want to put their Spinner as a child of HandleSSOCallback. If it's a render prop it would make more sense for it to be:
<HandleSSOCallback render={() => {
return (
<Card><CardBody><Spinner /></CardBody></Card>
)
}} />
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/react/src/components/HandleSSOCallback.tsx`:
- Around line 70-176: Add unit tests for the HandleSSOCallback component that
cover the sign-in/sign-up transfer and completion branches by mocking the Clerk
hooks and flows (useClerk, useSignIn, useSignUp) and asserting
navigation/finalize/setActive behavior: test when signIn.status === 'complete'
that signIn.finalize is called and navigateToApp is invoked; test
signUp.isTransferable path where signIn.create({transfer:true}) leads to
finalize or navigateToSignIn as appropriate; test signIn.isTransferable path
where signUp.create({transfer:true}) leads to finalize or navigateToSignUp; test
the needs_first_factor branch (when supportedFirstFactors do not all have
strategy === 'enterprise_sso') and the needs_second_factor/needs_new_password
branches call navigateToSignIn; test existingSession handling calls
clerk.setActive with sessionId and navigates to app; also include a render test
that returns the captcha div when no render prop provided. Mock and spy on
signIn.create, signIn.finalize, signUp.create, signUp.finalize, clerk.setActive
and the navigate callbacks to verify correct calls.
- Around line 77-164: HandleSSOCallback has no tests — add unit tests for the
component to cover the various SSO flows handled in its useEffect: signIn.status
=== 'complete' path (calls signIn.finalize -> navigateToApp),
signUp.isTransferable -> signIn.create then finalize or navigateToSignIn,
signIn.isTransferable -> signUp.create then finalize or navigateToSignUp,
needs_first_factor/needs_second_factor/needs_new_password -> navigateToSignIn,
and existingSession/session activation path (clerk.setActive -> navigateToApp);
in tests mock the clerk, signIn, signUp objects and assert the appropriate
methods (finalize, create, setActive) and navigation helpers (navigateToApp,
navigateToSignIn, navigateToSignUp) are called, and include at least one test
ensuring hasRun prevents re-running effect. Optionally, to silence
exhaustive-deps warnings in tests or code, either include the callback props
(navigateToApp/navigateToSignIn/navigateToSignUp) in the useEffect dependency
array or explicitly document/ignore the lint warning — but focus tests on
useEffect behaviour for functions useEffect, hasRun, clerk, signIn, signUp.
| export function HandleSSOCallback(props: HandleSSOCallbackProps): ReactNode { | ||
| const { navigateToApp, navigateToSignIn, navigateToSignUp, render } = props; | ||
| const clerk = useClerk(); | ||
| const { signIn } = useSignIn(); | ||
| const { signUp } = useSignUp(); | ||
| const hasRun = useRef(false); | ||
|
|
||
| useEffect(() => { | ||
| (async () => { | ||
| if (!clerk.loaded || hasRun.current) { | ||
| return; | ||
| } | ||
| // Prevent re-running this effect if the page is re-rendered during session activation (such as on Next.js). | ||
| hasRun.current = true; | ||
|
|
||
| // If this was a sign-in, and it's complete, there's nothing else to do. | ||
| // Note: We perform a cast | ||
| if ((signIn.status as string) === 'complete') { | ||
| await signIn.finalize({ | ||
| navigate: async (...params) => { | ||
| navigateToApp(...params); | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // If the sign-up used an existing account, transfer it to a sign-in. | ||
| if (signUp.isTransferable) { | ||
| await signIn.create({ transfer: true }); | ||
| if (signIn.status === 'complete') { | ||
| await signIn.finalize({ | ||
| navigate: async (...params) => { | ||
| navigateToApp(...params); | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
| // The sign-in requires additional verification, so we need to navigate to the sign-in page. | ||
| return navigateToSignIn(); | ||
| } | ||
|
|
||
| if ( | ||
| signIn.status === 'needs_first_factor' && | ||
| !signIn.supportedFirstFactors?.every(f => f.strategy === 'enterprise_sso') | ||
| ) { | ||
| // The sign-in requires the use of a configured first factor, so navigate to the sign-in page. | ||
| return navigateToSignIn(); | ||
| } | ||
|
|
||
| // If the sign-in used an external account not associated with an existing user, create a sign-up. | ||
| if (signIn.isTransferable) { | ||
| await signUp.create({ transfer: true }); | ||
| if (signUp.status === 'complete') { | ||
| await signUp.finalize({ | ||
| navigate: async (...params) => { | ||
| navigateToApp(...params); | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
| return navigateToSignUp(); | ||
| } | ||
|
|
||
| if (signUp.status === 'complete') { | ||
| await signUp.finalize({ | ||
| navigate: async (...params) => { | ||
| navigateToApp(...params); | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| if (signIn.status === 'needs_second_factor' || signIn.status === 'needs_new_password') { | ||
| // The sign-in requires a MFA token or a new password, so navigate to the sign-in page. | ||
| return navigateToSignIn(); | ||
| } | ||
|
|
||
| // The external account used to sign-in or sign-up was already associated with an existing user and active | ||
| // session on this client, so activate the session and navigate to the application. | ||
| if (signIn.existingSession || signUp.existingSession) { | ||
| const sessionId = signIn.existingSession?.sessionId || signUp.existingSession?.sessionId; | ||
| if (sessionId) { | ||
| // Because we're activating a session that's not the result of a sign-in or sign-up, we need to use the | ||
| // Clerk `setActive` API instead of the `finalize` API. | ||
| await clerk.setActive({ | ||
| session: sessionId, | ||
| navigate: async (...params) => { | ||
| return navigateToApp(...params); | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
| } | ||
| })(); | ||
| }, [clerk, signIn, signUp]); | ||
|
|
||
| if (render) { | ||
| return render(); | ||
| } | ||
|
|
||
| return ( | ||
| <div> | ||
| {/* Because a sign-in transferred to a sign-up might require captcha verification, make sure to render the | ||
| captcha element. */} | ||
| <div id='clerk-captcha'></div> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find tests that cover HandleSSOCallback
rg -n "HandleSSOCallback" --type ts --type tsx --type js --type jsx -g "*test*" -g "*spec*"Repository: clerk/javascript
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Find tests that cover HandleSSOCallback
rg -n "HandleSSOCallback" | grep -E "(test|spec)\.(ts|tsx|js|jsx)"Repository: clerk/javascript
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Search for HandleSSOCallback references in the codebase
rg "HandleSSOCallback" --max-count 20Repository: clerk/javascript
Length of output: 636
🏁 Script executed:
#!/bin/bash
# Find test files in packages/react
find packages/react -type f \( -name "*.test.*" -o -name "*.spec.*" \) | head -20Repository: clerk/javascript
Length of output: 1322
🏁 Script executed:
#!/bin/bash
# Check for HandleSSOCallback test file
find packages/react -name "*HandleSSOCallback*test*" -o -name "*HandleSSOCallback*spec*"Repository: clerk/javascript
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Search for HandleSSOCallback in all test files
find packages/react -path "*__tests__*" -type f \( -name "*.test.*" -o -name "*.spec.*" \) -exec grep -l "HandleSSOCallback" {} \;Repository: clerk/javascript
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check if HandleSSOCallback.tsx was modified in this PR
git diff --name-only | grep -i handlessocallbackRepository: clerk/javascript
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check the overall git status and what files were modified
git diff --name-only | head -20Repository: clerk/javascript
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check recent git history for HandleSSOCallback
git log --oneline --all -n 50 | head -20Repository: clerk/javascript
Length of output: 1527
🏁 Script executed:
#!/bin/bash
# Check if there are any references to HandleSSOCallback in existing test files
rg "HandleSSOCallback" packages/react/src/components/__tests__/Repository: clerk/javascript
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check the HandleSSOCallback.tsx file to understand its context
head -20 packages/react/src/components/HandleSSOCallback.tsxRepository: clerk/javascript
Length of output: 899
🏁 Script executed:
#!/bin/bash
# Check the git history for HandleSSOCallback specifically
git log --all --oneline -- packages/react/src/components/HandleSSOCallback.tsx | head -10Repository: clerk/javascript
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Check what changed in the recent commit for HandleSSOCallback
git show 9cf07019 --stat | grep -A 10 HandleSSOCallbackRepository: clerk/javascript
Length of output: 741
🏁 Script executed:
#!/bin/bash
# Check if tests were added for HandleSSOCallback in the same commit
git show 9cf07019 | grep -A 5 -B 5 "HandleSSOCallback.*test"Repository: clerk/javascript
Length of output: 42
🏁 Script executed:
#!/bin/bash
# List all files added in commit 9cf07019
git show 9cf07019 --name-only | grep -E "(test|spec)"Repository: clerk/javascript
Length of output: 35882
🏁 Script executed:
#!/bin/bash
# Specifically search for HandleSSOCallback in packages/react test directory
rg "HandleSSOCallback" packages/react/src/components/__tests__/Repository: clerk/javascript
Length of output: 42
Add tests for HandleSSOCallback component.
No test coverage found for this component. Add tests covering the sign-in/sign-up transfer branches and completion paths as required by coding guidelines.
🤖 Prompt for AI Agents
In `@packages/react/src/components/HandleSSOCallback.tsx` around lines 70 - 176,
Add unit tests for the HandleSSOCallback component that cover the
sign-in/sign-up transfer and completion branches by mocking the Clerk hooks and
flows (useClerk, useSignIn, useSignUp) and asserting
navigation/finalize/setActive behavior: test when signIn.status === 'complete'
that signIn.finalize is called and navigateToApp is invoked; test
signUp.isTransferable path where signIn.create({transfer:true}) leads to
finalize or navigateToSignIn as appropriate; test signIn.isTransferable path
where signUp.create({transfer:true}) leads to finalize or navigateToSignUp; test
the needs_first_factor branch (when supportedFirstFactors do not all have
strategy === 'enterprise_sso') and the needs_second_factor/needs_new_password
branches call navigateToSignIn; test existingSession handling calls
clerk.setActive with sessionId and navigates to app; also include a render test
that returns the captcha div when no render prop provided. Mock and spy on
signIn.create, signIn.finalize, signUp.create, signUp.finalize, clerk.setActive
and the navigate callbacks to verify correct calls.
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
| return navigateToSignIn(); | ||
| } | ||
|
|
||
| if ( |
There was a problem hiding this comment.
Just double checking - this navigates away unless ALL factors are enterprise_sso. Is that the intent?
There was a problem hiding this comment.
This is directly copy and pasted from our existing handleRedirectCallback implementation. It only navigates to sign-in if the status is needs_first_factor and there is at least one supported first-factor that's NOT enterprise_sso.
|
|
||
| // If this was a sign-in, and it's complete, there's nothing else to do. | ||
| // Note: We perform a cast | ||
| if ((signIn.status as string) === 'complete') { |
There was a problem hiding this comment.
// Note: We perform a cast - comment is cut off (i think?), should it explain why?
| } | ||
| } | ||
| })(); | ||
| }, [clerk, signIn, signUp]); |
There was a problem hiding this comment.
Navigate callbacks missing from deps array - intentional?
There was a problem hiding this comment.
Yes they're omitted since we're trying to avoid re-running this effect. We want to run this effect once once Clerk is loaded.
| captcha element. */} | ||
| <div id='clerk-captcha' /> | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
CodeRabbit flagged this too - no test coverage for this component
There was a problem hiding this comment.
Is this something e2e tests are better suited for?
There was a problem hiding this comment.
given the number of permutations I actually think a unit test with mocks might be better. I'll add that!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/react/src/components/HandleSSOCallback.tsx`:
- Around line 78-166: The useEffect in HandleSSOCallback.tsx currently checks
clerk.loaded but doesn't include it in the dependency array, so update the
dependency array on the useEffect that contains the async IIFE (the effect
referencing signIn, signUp and clerk) to include clerk.loaded (e.g., change
[clerk, signIn, signUp] to include clerk.loaded) so the effect re-runs when
Clerk finishes loading; also add unit/interaction tests for HandleSSOCallback to
cover the flow when clerk.loaded transitions from false to true to ensure the
SSO logic runs as expected.
| useEffect(() => { | ||
| (async () => { | ||
| if (!clerk.loaded || hasRun.current) { | ||
| return; | ||
| } | ||
| // Prevent re-running this effect if the page is re-rendered during session activation (such as on Next.js). | ||
| hasRun.current = true; | ||
|
|
||
| // If this was a sign-in, and it's complete, there's nothing else to do. | ||
| // Note: We perform a cast here to prevent TypeScript from narrowing the type of signIn.status. TypeScript | ||
| // doesn't understand that the status can be mutated during the execution of this function. | ||
| if ((signIn.status as string) === 'complete') { | ||
| await signIn.finalize({ | ||
| navigate: async (...params) => { | ||
| navigateToApp(...params); | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // If the sign-up used an existing account, transfer it to a sign-in. | ||
| if (signUp.isTransferable) { | ||
| await signIn.create({ transfer: true }); | ||
| if (signIn.status === 'complete') { | ||
| await signIn.finalize({ | ||
| navigate: async (...params) => { | ||
| navigateToApp(...params); | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
| // The sign-in requires additional verification, so we need to navigate to the sign-in page. | ||
| return navigateToSignIn(); | ||
| } | ||
|
|
||
| if ( | ||
| signIn.status === 'needs_first_factor' && | ||
| !signIn.supportedFirstFactors?.every(f => f.strategy === 'enterprise_sso') | ||
| ) { | ||
| // The sign-in requires the use of a configured first factor, so navigate to the sign-in page. | ||
| return navigateToSignIn(); | ||
| } | ||
|
|
||
| // If the sign-in used an external account not associated with an existing user, create a sign-up. | ||
| if (signIn.isTransferable) { | ||
| await signUp.create({ transfer: true }); | ||
| if (signUp.status === 'complete') { | ||
| await signUp.finalize({ | ||
| navigate: async (...params) => { | ||
| navigateToApp(...params); | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
| return navigateToSignUp(); | ||
| } | ||
|
|
||
| if (signUp.status === 'complete') { | ||
| await signUp.finalize({ | ||
| navigate: async (...params) => { | ||
| navigateToApp(...params); | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| if (signIn.status === 'needs_second_factor' || signIn.status === 'needs_new_password') { | ||
| // The sign-in requires a MFA token or a new password, so navigate to the sign-in page. | ||
| return navigateToSignIn(); | ||
| } | ||
|
|
||
| // The external account used to sign-in or sign-up was already associated with an existing user and active | ||
| // session on this client, so activate the session and navigate to the application. | ||
| if (signIn.existingSession || signUp.existingSession) { | ||
| const sessionId = signIn.existingSession?.sessionId || signUp.existingSession?.sessionId; | ||
| if (sessionId) { | ||
| // Because we're activating a session that's not the result of a sign-in or sign-up, we need to use the | ||
| // Clerk `setActive` API instead of the `finalize` API. | ||
| await clerk.setActive({ | ||
| session: sessionId, | ||
| navigate: async (...params) => { | ||
| return navigateToApp(...params); | ||
| }, | ||
| }); | ||
| return; | ||
| } | ||
| } | ||
| })(); | ||
| }, [clerk, signIn, signUp]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the hook implementations to check if they return stable references
find packages/react/src/hooks -type f \( -name "*.ts" -o -name "*.tsx" \) | head -20Repository: clerk/javascript
Length of output: 609
🏁 Script executed:
#!/bin/bash
# Check the implementation of useClerk hook
rg -n "export.*useClerk\s*=" packages/react/src -A 10 -B 2Repository: clerk/javascript
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Look for test files for HandleSSOCallback
find . -type f \( -name "*HandleSSOCallback*" -o -path "*/test/*" -o -path "*/tests/*" \) | grep -i "sso\|callback"Repository: clerk/javascript
Length of output: 113
🏁 Script executed:
#!/bin/bash
# Check if there are any test files in the same directory as HandleSSOCallback
ls -la packages/react/src/components/ | grep -i testRepository: clerk/javascript
Length of output: 119
🏁 Script executed:
#!/bin/bash
# Search for jest or test config to understand testing setup
find packages/react -type f \( -name "*.test.ts" -o -name "*.test.tsx" -o -name "*.spec.ts" -o -name "*.spec.tsx" \) | head -10Repository: clerk/javascript
Length of output: 700
🏁 Script executed:
#!/bin/bash
# Search for where useClerk is defined or exported
rg "useClerk" packages/react/src/hooks --max-count 5Repository: clerk/javascript
Length of output: 692
🏁 Script executed:
#!/bin/bash
# Check the hooks index to see what's exported
cat packages/react/src/hooks/index.tsRepository: clerk/javascript
Length of output: 545
🏁 Script executed:
#!/bin/bash
# Read the full HandleSSOCallback component to understand context
cat packages/react/src/components/HandleSSOCallback.tsxRepository: clerk/javascript
Length of output: 6220
🏁 Script executed:
#!/bin/bash
# Check if there's a test file specifically for HandleSSOCallback
ls -la packages/react/src/components/__tests__/ | grep -i "sso\|callback"Repository: clerk/javascript
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Search for how useClerk is typically used in other components to understand stability
rg "useClerk\(\)" packages/react/src/components -A 2 | head -30Repository: clerk/javascript
Length of output: 311
🏁 Script executed:
#!/bin/bash
# Find the shared package location
find . -type d -name shared | grep -E "packages/shared|@clerk/shared" | head -5Repository: clerk/javascript
Length of output: 77
🏁 Script executed:
#!/bin/bash
# Look for useClerk implementation in the codebase
rg "export.*function useClerk|export const useClerk" -g "*.ts" -g "*.tsx" -A 10Repository: clerk/javascript
Length of output: 10049
🏁 Script executed:
#!/bin/bash
# Check for memoization in hook implementations - look for useMemo wrapping
rg "useMemo.*useClerk|useClerk.*useMemo" packages/ -g "*.ts" -g "*.tsx"Repository: clerk/javascript
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check the clerk object structure to understand what clerk.loaded is
rg "loaded.*:" packages/shared -g "*.ts" -g "*.tsx" -B 2 -A 2 | head -50Repository: clerk/javascript
Length of output: 4252
🏁 Script executed:
#!/bin/bash
# Check how useClerk is typically memoized or cached in tests
find packages -name "*.test.tsx" -o -name "*.test.ts" | xargs grep -l "useClerk" | head -1 | xargs cat | head -100Repository: clerk/javascript
Length of output: 2170
🏁 Script executed:
#!/bin/bash
# Find the useClerkInstanceContext implementation
rg "useClerkInstanceContext" packages/shared/src -A 10 -B 2Repository: clerk/javascript
Length of output: 49661
🏁 Script executed:
#!/bin/bash
# Check if there's a ClerkProvider that wraps the context
rg "ClerkProvider|ClerkInstanceContext" packages/shared/src -A 5 -B 2 | head -80Repository: clerk/javascript
Length of output: 5548
🏁 Script executed:
#!/bin/bash
# Check how the clerk context value is created to verify stability
rg "Provider.*value.*clerk|clerk.*Provider.*value" packages/ -g "*.tsx" -g "*.ts" -A 3Repository: clerk/javascript
Length of output: 1226
Add clerk.loaded to the useEffect dependency array to ensure the effect reruns when Clerk finishes loading.
The effect currently guards on !clerk.loaded but doesn't include it in dependencies [clerk, signIn, signUp]. If the clerk object reference remains stable, the effect will skip on first render when clerk.loaded is false and never rerun when it becomes true, causing the entire SSO flow to be skipped.
Additionally, tests should be added to cover this component's behavior.
🤖 Prompt for AI Agents
In `@packages/react/src/components/HandleSSOCallback.tsx` around lines 78 - 166,
The useEffect in HandleSSOCallback.tsx currently checks clerk.loaded but doesn't
include it in the dependency array, so update the dependency array on the
useEffect that contains the async IIFE (the effect referencing signIn, signUp
and clerk) to include clerk.loaded (e.g., change [clerk, signIn, signUp] to
include clerk.loaded) so the effect re-runs when Clerk finishes loading; also
add unit/interaction tests for HandleSSOCallback to cover the flow when
clerk.loaded transitions from false to true to ensure the SSO logic runs as
expected.
Description
This PR adds a new
HandleSSOCallbackcomponent which wraps our custom ui flow for handling transferrable sign-ins and sign-ups. This component packages the specific logic that we use to handle that flow in our own components; customers will still be able to implement their own specific custom flow, but this component is being made available to make that a little easier.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.